Skip to content

feat(bitbucketdatacenter): allow service accounts to not require user setup#2726

Merged
zakisk merged 1 commit into
tektoncd:mainfrom
Ru13en:allow-project-repo-http-tokens
Jun 9, 2026
Merged

feat(bitbucketdatacenter): allow service accounts to not require user setup#2726
zakisk merged 1 commit into
tektoncd:mainfrom
Ru13en:allow-project-repo-http-tokens

Conversation

@Ru13en

@Ru13en Ru13en commented May 13, 2026

Copy link
Copy Markdown
Contributor

📝 Description of the Change

Previously, using project or repository HTTP scoped tokens required configuring an associated user, even when the token already provided the necessary access context.
This PR removes the requirement to configure a user when using HTTP tokens from project and repository scopes.
It updates authentication flow to rely directly on the scoped token context, when only token is provided.
Related validation and tests were adjusted accordingly

🔗 Linked GitHub Issue

Fixes #
#2685

🧪 Testing Strategy

  • Unit tests
  • Integration tests
  • End-to-end tests
  • Manual testing
  • Not Applicable

🤖 AI Assistance

AI assistance can be used for various tasks, such as code generation,
documentation, or testing.

Please indicate whether you have used AI assistance
for this PR and provide details if applicable.

  • I have not used any AI assistance for this PR.
  • I have used AI assistance for this PR.

Important

Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.

If the majority of the code in this PR was generated by an AI, please add a Co-authored-by trailer to your commit message.
For example:

Co-authored-by: Claude noreply@anthropic.com

✅ Submitter Checklist

  • 📝 My commit messages are clear, informative, and follow the project's How to write a git commit message guide. The Gitlint linter ensures in CI it's properly validated
  • ✨ I have ensured my commit message prefix (e.g., fix:, feat:) matches the "Type of Change" I selected above.
  • ♽ I have run make test and make lint locally to check for and fix any
    issues. For an efficient workflow, I have considered installing
    pre-commit and running pre-commit install to
    automate these checks.
  • 📖 I have added or updated documentation for any user-facing changes.
  • 🧪 I have added sufficient unit tests for my code changes.
  • 🎁 I have added end-to-end tests where feasible. See README for more details.
  • 🔎 I have addressed any CI test flakiness or provided a clear reason to bypass it.
  • If adding a provider feature, I have filled in the following and updated the provider documentation:
    • GitHub App
    • GitHub Webhook
    • Gitea/Forgejo
    • GitLab
    • Bitbucket Cloud
    • Bitbucket Data Center

@linux-foundation-easycla

linux-foundation-easycla Bot commented May 13, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: Ru13en / name: Ruben Rodrigues (afb412f)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Bitbucket Data Center provider to allow authentication without an explicitly defined user by falling back to a direct repository URL request for token validation. Feedback from the review highlights a critical need for a nil check on the repository object to prevent runtime panics. Furthermore, the current error handling logic needs refinement to avoid malformed error strings when wrapping nil errors and to provide more accurate messaging when the user field is empty.

Comment thread pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go
Comment thread pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go
Comment thread pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go Outdated
Comment thread pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go Outdated
@Ru13en Ru13en force-pushed the allow-project-repo-http-tokens branch 2 times, most recently from 645ee48 to afb412f Compare May 13, 2026 20:53
@Ru13en Ru13en changed the title bitbucketdatacenter: allow service accounts to not require user setup feat(bitbucketdatacenter): allow service accounts to not require user setup May 13, 2026
@Ru13en Ru13en requested a review from mathur07 May 13, 2026 21:12

@mathur07 mathur07 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zakisk

zakisk commented May 14, 2026

Copy link
Copy Markdown
Member

/ok-to-test

@codecov-commenter

codecov-commenter commented May 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 59.73%. Comparing base (402d5c7) to head (01593b5).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
pkg/provider/bitbucketdatacenter/test/test.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2726   +/-   ##
=======================================
  Coverage   59.73%   59.73%           
=======================================
  Files         210      210           
  Lines       21112    21117    +5     
=======================================
+ Hits        12611    12615    +4     
- Misses       7706     7707    +1     
  Partials      795      795           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zakisk

zakisk commented May 19, 2026

Copy link
Copy Markdown
Member

@Ru13en what do you mean by service account? do you mean repo http access token as you added in PR description?

@Ru13en

Ru13en commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@zakisk, correct. Bitbucket DC has 3 types of HTTPS tokens. User tokens have the authentication attached to the Licensed user. Repository HTTPS token and Project HTTPS have the authentication integrated within the server and can be used as Service Account. There is no need to have an extra Bitbucket User license, when the Project and Repo token can create webooks, create repositories (for project token), commits and comments.
Example of a comment made via a MLOPS Project HTTP token:
image

@zakisk

zakisk commented May 25, 2026

Copy link
Copy Markdown
Member

@Ru13en I've tested this and found that you can actually use project access token with admin permission in PaC without any code changes but for repo access token we need to LICENCED_USER for checking permission of the user pushing to repo or raising PR.

@Ru13en

Ru13en commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@Ru13en I've tested this and found that you can actually use project access token with admin permission in PaC without any code changes but for repo access token we need to LICENCED_USER for checking permission of the user pushing to repo or raising PR.

@zakisk HTTP repository tokens with admin permission can get the members of the group necessary to check the permissions that you mention. Try with the following command:

curl -sS -H "Authorization: Bearer $REPO_ADMIN_TOKEN" <bitbucket_base_url>/rest/api/1.0/admin/groups/more-members?context=<Group_name>

Also they can get all permissions of the repository that it belongs.

curl -sS -H "Authorization: Bearer $REPO_ADMIN_TOKEN" <bitbucket_base_url>/rest/api/1.0/projects/{project_key}/repos/{repository_slug}/permissions/search

When you setup the scm-go client, if you don't drop the user, you will be forced to add an known user as a placeholder for the Repository PAC config and then this username somehow is being used...

@zakisk

zakisk commented May 26, 2026

Copy link
Copy Markdown
Member

@Ru13en yeah, you're right I tried it. but you don't need to do the changes you're doing at the moment. it's issue in ACL when org membership check fails due to lack of permission on repo http token, it's returning right from there without checking below repo collaborator permission so you can just have a condition like this to get repo token working fine:

allowed, resp, err := v.Client().Organizations.IsMember(ctx, event.Organization, event.Sender)
if err != nil && resp != nil && resp.Status != http.StatusUnauthorized {
	return false, err
}

but your token must be having repo admin permission

@Ru13en

Ru13en commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

@zakisk these changes are only to drop the unnecessary requirement for a username if I provide a HTTP Token during the PaC Repository configuration, since you are forced to add a valid one.
For security reasons we don't want to associate users or use them as placeholders with Admin tokens.
In this PR I didn't addressed any issue related with the lack of permissions.

@zakisk

zakisk commented May 26, 2026

Copy link
Copy Markdown
Member

but if its working with repo and project access token why do you wanna remove user account check?

@Ru13en

Ru13en commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

When you add the Repo config via GitOps we use External Secrets to inject the HTTP Tokens.
Right now we need to add a Valid Placeholder user.
We can't leave it empty or we can't add anything random.
Github configs don't need the username, why Bitbucket datacenter HTTP tokens require it?
We don't want to associate users or use them as placeholders with Admin tokens, we have a flag in the our security audits to drop such association.

Comment thread pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go Outdated
@zakisk

zakisk commented May 26, 2026

Copy link
Copy Markdown
Member

why Bitbucket datacenter HTTP tokens require it?

it was implemented a while ago in this commit to ensure that token is valid 549b2d8

Comment thread pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go Outdated
@zakisk

zakisk commented May 26, 2026

Copy link
Copy Markdown
Member

When you add the Repo config via GitOps we use External Secrets to inject the HTTP Tokens. Right now we need to add a Valid Placeholder user. We can't leave it empty or we can't add anything random. Github configs don't need the username, why Bitbucket datacenter HTTP tokens require it? We don't want to associate users or use them as placeholders with Admin tokens, we have a flag in the our security audits to drop such association.

it makes sense after you explained your use case!

@zakisk zakisk force-pushed the allow-project-repo-http-tokens branch from afb412f to 279d636 Compare May 26, 2026 09:58
@zakisk

zakisk commented May 26, 2026

Copy link
Copy Markdown
Member

/ok-to-test

Comment thread docs/content/docs/providers/bitbucket-datacenter.md Outdated
@zakisk

zakisk commented Jun 8, 2026

Copy link
Copy Markdown
Member

@Ru13en please refine docs changes and we're good to go with it...

@Ru13en Ru13en force-pushed the allow-project-repo-http-tokens branch 2 times, most recently from 567d993 to 4c61450 Compare June 8, 2026 14:14
@Ru13en

Ru13en commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Made the changes as requested @zakisk
hope that is good enough.
BR

@Ru13en Ru13en force-pushed the allow-project-repo-http-tokens branch 2 times, most recently from d21c0db to f1c5842 Compare June 8, 2026 15:11
@mathur07

mathur07 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

/agentic_review

@zakisk

zakisk commented Jun 9, 2026

Copy link
Copy Markdown
Member

/ok-to-test

@zakisk zakisk force-pushed the allow-project-repo-http-tokens branch from f1c5842 to ceb5666 Compare June 9, 2026 11:22
@zakisk

zakisk commented Jun 9, 2026

Copy link
Copy Markdown
Member

/ok-to-test

@zakisk

zakisk commented Jun 9, 2026

Copy link
Copy Markdown
Member

too many E2E failures... 😕 @Ru13en BTW, it's not your PR. we're working for tests fixing

@zakisk zakisk force-pushed the allow-project-repo-http-tokens branch from ceb5666 to 0df0db6 Compare June 9, 2026 12:02
@zakisk

zakisk commented Jun 9, 2026

Copy link
Copy Markdown
Member

/ok-to-test

@zakisk zakisk force-pushed the allow-project-repo-http-tokens branch from 0df0db6 to 01593b5 Compare June 9, 2026 15:40
@zakisk

zakisk commented Jun 9, 2026

Copy link
Copy Markdown
Member

/ok-to-test

@zakisk

zakisk commented Jun 9, 2026

Copy link
Copy Markdown
Member

/retest

@zakisk zakisk merged commit cbd582d into tektoncd:main Jun 9, 2026
17 of 26 checks passed
@zakisk

zakisk commented Jun 9, 2026

Copy link
Copy Markdown
Member

@Ru13en Thank you for contributing! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants